Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Veras Guess Who! #338

Open
wants to merge 13 commits into
base: main
Choose a base branch
from
Open

Veras Guess Who! #338

wants to merge 13 commits into from

Conversation

verawitting
Copy link

No description provided.

@LauraLyckholm
Copy link

Whoopsie, I don't seem to be able to guess the character! The win or lose page doesn't show up, and error in log says "userName is not defined". Could you look at this before I continue my review? 🥰

Comment on lines +82 to +90
<h2>Your time:
<span class="timer" id="timer">0</span> seconds
</h2>
</section>

<section class="count-section">
<h2>Number of guesses:
<span class="guess-container" id="guess-container">0</span>
</h2>

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Love that you have added a timer and number of guesses! It made me really feel the pressure 😅 As a way of making things more "complex" next time you could try and add these part through Javascript? Just if you want to try and evolve the app 🥰

code/index.html Outdated
Comment on lines 108 to 113
<section class="count-section">
<h2>Your number of guesses was:
<span class="guess-container" id="win-or-lose-guess-container">0</span>
</h2>
</section>
<h2 id ="winOrLoseTimer"></h2>
Copy link

@LauraLyckholm LauraLyckholm Sep 12, 2023

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

It's a great addition to show the user how many guesses they ended up making once the game has finished, great job! As a way of evolving the code next time you could try and add these parts through JS as well? But the Win or lose page looks amazing! Seeing the image of the guess there is also a great addition!

code/script.js Outdated
const winOrLose = document.getElementById('winOrLose')
const winOrLoseText = document.getElementById('winOrLoseText')
const playAgainButton = document.getElementById('playAgain')
const winnerLoserImg = document.createElement('img');

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

You create a new element here, good job!

//Setting children to the winOrLose element
winOrLose.appendChild(winnerLoserImg);
winOrLose.appendChild(winnerLoserP);


// Array with all the characters, as objects
const CHARACTERS = [

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I really do love that you changed the characters for your own, it makes the game so much more interesting to play! It's a big job to do this all manually. We wish we would have known how to do a fetch here right? 😅 Good job!


// Draw the game board
const generateBoard = () => {
board.innerHTML = ''
charactersInPlay.forEach((person) => {
board.innerHTML += `
<div class="card">
<div class="card" style="background-image: url('${person.img}'); background-image: linear-gradient(transparent.rgba(0,0,0,10.6));">

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Nice addition of styling here! Fun 😍

Comment on lines +301 to +310
const startTimer = () => {
timerInterval = setInterval(() => {
seconds++;
timerElement.innerText = seconds;
}, 1000); // updates the timer every second!
}
//This function stops the timer! And gives the elapsed time!
const stopTimer = () => {
clearInterval(timerInterval);
return seconds;

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This is a great addition, great to see you have implemented a timer!

Comment on lines 340 to 360
if (category === 'hair' || category === 'eyes') {

//to see that value is the same, then keep is loaded with true, invoke next func :)
if (secret[category] === value) {
keep = true
filterCharacters(true);
} //to see that value is NOT same, then keep is loaded with false, invoke next func :)
else {
keep = false
filterCharacters(false);
}
} else if (category === 'accessories' || category === 'other') {

//to se if object has any value set at all for that question, true of false.
if (secret[category].includes(value)) {
keep = true
filterCharacters(true);
} else {
keep = false
filterCharacters(false);
}
}
}

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Nice if/else, short and concise, makes the code look clean and the comments help the reader understand what is going on 👊🏻

// 2. Set a Message to show in the win or lose section accordingly
// 3. Show the win or lose section
// 4. Hide the game board
const checkMyGuess = (personToCheck, elapsedTime) => {

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Great job placing the elapsedTime as a parameter here!


// All the event listeners
restartButton.addEventListener('click', start)
startButton.addEventListener('click', () => {
startBox.style.display = 'none'; // Hide the start box

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This is such a fun addition, I love it! You're showing great creativity and also nice that you figured out how to do this in JS. Good job!

@@ -1,7 +1,7 @@
/* Global css variables used for colors */

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I like that you have added you own styling to the project making it more personal!

Copy link

@LauraLyckholm LauraLyckholm left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Hi Vera! I love your Guess Who game! So many interesting and fun characters, the list of properties to choose from is very intricate and the timer and guesses made functions added makes this really feel like a complete game - I can only say amazing job! You have definitely fulfilled the general requirements, as well as fulfilled the intermediate stretch goals. The timer from the advanced go als is also there!

The code works and the game is fun to play. Great job! The only thing I would suggest regarding the restart function is to separate it from the start button, and perhaps while doing that making the restart look different than the start. Now, after the game is restarted the same welcome-window appears, It would just add an extra sprinkle if that was a bit different 🥰

Keep up the good work!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants